repository_ctx: Allow renaming archive entries during extraction. #16052

jmillikin · 2022-08-05T12:25:27Z

Adds a rename_files= parameter to the extract() and download_and_extract() methods of repository_ctx. This new parameter takes a dict of archive entries to rename, and is applied prior to any prefix adjustment.

jmillikin · 2022-08-05T12:32:12Z

Use cases include:

Archives where multiple files have the same name in different case. On platforms with case-insensitive filesystems (macOS, Windows) these archives can't be extracted as-is, and the files can't be fixed up after extraction.
- One such project is the Linux kernel, which has files named like net/netfilter/xt_dscp.c and net/netfilter/xt_DSCP.c (among others).
Archives containing non-UTF8 filenames fail to extract on platforms or filesystems that enforce Unicode paths.
- See Cannot successfully extract go_sdk because of unicode filename bazel-contrib/rules_go#2771 (Go toolchains from golang.org fail to extract)

A longer example of extracting the Linux kernel on macOS (which is case-insensitive by default):

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

def _linux_repository(ctx):
    ctx.file("WORKSPACE", "workspace(name = {name})\n".format(
        name = repr(ctx.name),
    ))
    ctx.file("BUILD.bazel", """
genrule(
    name = "nf_srcs",
    srcs = glob(["net/netfilter/*"]),
    outs = ["nf_srcs.txt"],
    cmd = "ls -1 external/linux/net/netfilter/ > $@",
)
""")

    ctx.download_and_extract(
        url = ["https://cdn.kernel.org/pub/linux/kernel/v5.x/linux-5.19.tar.xz"],
        sha256 = "ff240c579b9ee1affc318917de07394fc1c3bb49dac25ec1287370c2e15005a8",
        stripPrefix = "linux-5.19",
        rename_files = _rename_files("5.19"),
    )

def _rename_files(version):
    netfilter_srcs = {
        "xt_DSCP.c": "xt_DSCP_target.c",
        "xt_HL.c": "xt_HL_target.c",
        "xt_RATEEST.c": "xt_RATEEST_target.c",
        "xt_TCPMSS.c": "xt_TCPMSS_target.c",
    }
    renames = {}
    for orig_name, new_name in netfilter_srcs.items():
        prefix = "linux-{}/net/netfilter/".format(version)
        renames[prefix + orig_name] = prefix + new_name
    # and similar logic for netfilter headers under include/uapi/linux/
    return renames

linux_repository = repository_rule(
    implementation = _linux_repository,
)

$ tar -tf ~/downloads/linux-5.19.tar.xz | grep -i xt_dscp.c
linux-5.19/net/netfilter/xt_DSCP.c
linux-5.19/net/netfilter/xt_dscp.c
$ bazel build @linux//:nf_srcs
[...]
Target @linux//:nf_srcs up-to-date:
  bazel-bin/external/linux/nf_srcs.txt
$ cat bazel-bin/external/linux/nf_srcs.txt | grep -i xt_dscp
xt_DSCP_target.c
xt_dscp.c

fmeum · 2022-08-05T13:21:38Z

.../java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java

@@ -749,8 +749,22 @@ public StructImpl download(
                    + " archive. Instead of needing to specify this prefix over and over in the"
                    + " <code>build_file</code>, this field can be used to strip it from extracted"
                    + " files."),
+        @Param(
+            name = "renameFiles",


This is the first time I actually noticed that stripPrefix has a camel case name - that looks like a bug as Starlark parameters usually use snake case. Using snake case for renameFiles at least wouldn't complicate future efforts to clean up this inconsistency.

jmillikin · 2022-08-20T02:41:47Z

Gentle ping

Wyverald

thanks!

Wyverald · 2022-08-22T11:51:44Z

src/main/java/com/google/devtools/build/lib/bazel/repository/DecompressorDescriptor.java

  private final Decompressor decompressor;

  private DecompressorDescriptor(
      String targetKind, String targetName, Path archivePath, Path repositoryPath,
-      @Nullable String prefix, boolean executable, Decompressor decompressor) {
+      @Nullable String prefix, boolean executable,
+      @Nullable Map<String, String> renameFiles,


this doesn't need to be nullable (it's never null anyway, and null has the same semantics as empty). also remove all the null checks

Wyverald · 2022-08-22T11:52:03Z

src/main/java/com/google/devtools/build/lib/bazel/repository/DecompressorDescriptor.java

@@ -164,6 +175,12 @@ public Builder setExecutable(boolean executable) {
      return this;
    }

+    @CanIgnoreReturnValue
+    public Builder setRenameFiles(Map<String, String> renameFiles) {
+      this.renameFiles = renameFiles;


defensively copy here (ImmutableMap.copyOf)

Wyverald · 2022-08-22T11:53:09Z

.../java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java

+            doc =
+                "An optional dict specifying files to rename during the extraction. Archive entries"
+                    + " with names exactly matching a key will be renamed to the value, prior to"
+                    + " any directory prefix adjustment."),


could you add more information on how this parameter can be used, like you did in the PR comments? or maybe even link to this PR

Wyverald · 2022-08-22T11:53:30Z

.../java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java

+            doc =
+                "An optional dict specifying files to rename during the extraction. Archive entries"
+                    + " with names exactly matching a key will be renamed to the value, prior to"
+                    + " any directory prefix adjustment."),


Wyverald · 2022-08-22T11:54:59Z

src/test/java/com/google/devtools/build/lib/bazel/repository/CompressedTarFunctionTest.java

+   * extraction process.
+   */
+  @Test
+  public void testDecompressWithRenamedFiles() throws Exception {


can we add a test case where strip_prefix is also present, to test that rename_files happens before strip_prefix?

Wyverald

thanks!

Adds a `rename_files=` parameter to the `extract()` and `download_and_extract()` methods of `repository_ctx`. This new parameter takes a dict of archive entries to rename, and is applied prior to any prefix adjustment. Closes bazelbuild#16052. PiperOrigin-RevId: 469686270 Change-Id: Ia8bc37ba1150ee1ece8ef4f613c26d0f93ca988b

repository_ctx: Allow renaming archive entries during extraction.

429558c

jmillikin force-pushed the ctx-extract-rename-outputs branch from 2856e8d to 429558c Compare August 5, 2022 12:47

fmeum reviewed Aug 5, 2022

View reviewed changes

renameFiles -> rename_files

d408e4c

sgowroji added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. awaiting-review PR is awaiting review from an assigned reviewer labels Aug 5, 2022

Wyverald requested changes Aug 22, 2022

View reviewed changes

jmillikin added 2 commits August 23, 2022 23:52

review feedback

853fe1c

Add tests for combination of entry renaming and prefix stripping.

ffe6a6b

Wyverald approved these changes Aug 23, 2022

View reviewed changes

Wyverald added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Aug 23, 2022

copybara-service bot closed this in 2b02416 Aug 24, 2022

sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Aug 24, 2022

jayconrod mentioned this pull request May 17, 2023

go_download_sdk: apply extraction workaround to zips on non-windows OSs bazel-contrib/rules_go#3563

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

repository_ctx: Allow renaming archive entries during extraction. #16052

repository_ctx: Allow renaming archive entries during extraction. #16052

jmillikin commented Aug 5, 2022 •

edited

Loading

jmillikin commented Aug 5, 2022 •

edited

Loading

fmeum Aug 5, 2022

jmillikin Aug 5, 2022

jmillikin commented Aug 20, 2022

Wyverald left a comment

Wyverald Aug 22, 2022

jmillikin Aug 23, 2022

Wyverald Aug 22, 2022

jmillikin Aug 23, 2022

Wyverald Aug 22, 2022

jmillikin Aug 23, 2022

Wyverald Aug 22, 2022

jmillikin Aug 23, 2022

Wyverald Aug 22, 2022

jmillikin Aug 23, 2022

Wyverald left a comment

repository_ctx: Allow renaming archive entries during extraction. #16052

repository_ctx: Allow renaming archive entries during extraction. #16052

Conversation

jmillikin commented Aug 5, 2022 • edited Loading

jmillikin commented Aug 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmillikin commented Aug 20, 2022

Wyverald left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wyverald left a comment

Choose a reason for hiding this comment

jmillikin commented Aug 5, 2022 •

edited

Loading

jmillikin commented Aug 5, 2022 •

edited

Loading